Skip to content

GAUD-10597 - Add drag test command - #1069

Merged
svanherk merged 4 commits into
mainfrom
GAUD-10597-Add-drag-helper
Aug 27, 2026
Merged

GAUD-10597 - Add drag test command#1069
svanherk merged 4 commits into
mainfrom
GAUD-10597-Add-drag-helper

Conversation

@svanherk

Copy link
Copy Markdown
Contributor

Not really tied to how this works at all, so figured I'd get it up and get feedback rather than trying to keep tweaking it.

@svanherk
svanherk requested a review from a team as a code owner August 26, 2026 22:29
Comment thread src/browser/commands.js Outdated
}

export async function dragElemBy(elem, offsetX = 0, offsetY = 0) {
const pixels = 10; // Mimic dragging by moving in 10px increments to the target position

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to simulate a user actually dragging something, which fires events every few pixels. Just moving from one place to another programmatically doesn't fire any intermediate values.

I'm firing every 10px, but this could also just break up the values passed into 5 steps of x pixels. That's likely better for large numbers, whereas this is better for small number (eg not bothering to divide a move of 2 pixels).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe call this something like dragIncrementSize?

Comment thread src/browser/commands.js
await sendMouse({ type: 'up' });
}

export async function dragElemBy(elem, offsetX = 0, offsetY = 0) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name is a little odd. I'm trying to follow the pattern of the other helpers in this file, taking x and y offsets as parameters. They work a little differently though, and use "at" terminology, which doesn't make sense here.

But dragElemTo isn't correct, unless I take an x and y location rather than an offset. This is nicer for the helper, worse for the consumer - it's much easier if I can say "Drag it back 5 pixels" instead of "Drag it to spot x = 250".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

Comment thread test/browser/commands.test.js Outdated

@dlockhart dlockhart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

Comment thread src/browser/commands.js
await sendMouse({ type: 'up' });
}

export async function dragElemBy(elem, offsetX = 0, offsetY = 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

Comment thread src/browser/commands.js Outdated
}

export async function dragElemBy(elem, offsetX = 0, offsetY = 0) {
const pixels = 10; // Mimic dragging by moving in 10px increments to the target position

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe call this something like dragIncrementSize?

Comment thread test/browser/commands.test.js Outdated
Comment thread src/browser/commands.js
const dy = Math.sign(offsetY) * Math.min(Math.abs(offsetY), pixels * i);
await sendMouse({ type: 'move', position: [position.x + dx, position.y + dy] });
}
await sendMouse({ type: 'up' });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm: I'm assuming there's something already that'll release the mouse in between tests, just in case this aborts partway through?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good callout - we actually don't. @web/test-runner-commands has a resetMouse command that moves it back to (0,0) and releases buttons, but we don't use it. We just do the move ourselves. Been like that since the get-go and no clues in the PR discussion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what @web/test-runner-commands's does, I think we just try to use it...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh cool, yeah let's try using that!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this PR merges, I'll check test run times and see if it makes a huge difference. I don't think it will, since we only reset the mouse if we've touched it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I don't think we can use resetMouse. I'm running into this issue: modernweb-dev/web#2085. When --watching in Chromium, every time the mouse is reset, the context menu is opened and stays open for the next test (unless you click to close it, and interfere with the test). Doesn't matter if you have devTools open or not.

image

I think we just do it ourselves until this is fixed, and only bother with the left button, since we don't expose any commands that press the middle or right buttons anyways.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to do this in a separate PR, that also adds the ability to not complete the drag.

@svanherk
svanherk merged commit fcdb02b into main Aug 27, 2026
7 checks passed
@svanherk
svanherk deleted the GAUD-10597-Add-drag-helper branch August 27, 2026 17:03
@d2l-github-release-tokens

Copy link
Copy Markdown

🎉 This PR is included in version 1.51.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants